Skip to content

Handle rule filter serialization#469

Merged
nilmerg merged 5 commits into
mainfrom
rule-filter-handling
May 22, 2026
Merged

Handle rule filter serialization#469
nilmerg merged 5 commits into
mainfrom
rule-filter-handling

Conversation

@BastianLedererIcinga
Copy link
Copy Markdown
Contributor

@BastianLedererIcinga BastianLedererIcinga commented Apr 28, 2026

resolve #466

Introduce RuleSerializer to create Json from a configured filter.

Introduce Icinga\Module\Notifications\Hook\V2 which no longer requires serializeRuleFilter() and getRuleFilterTargets() as they are now obsolete.

Adjust EventRuleController to skip target selection use the new RuleSerializer.

require Icinga/ipl-web#380
require Icinga/ipl-web#383
require Icinga/ipl-stdlib#75
require Icinga/ipl-stdlib#76

@cla-bot cla-bot Bot added the cla/signed CLA is signed by all contributors of a PR label Apr 28, 2026
@BastianLedererIcinga BastianLedererIcinga force-pushed the rule-filter-handling branch 2 times, most recently from e3da66a to b8a38fc Compare April 28, 2026 08:02
@BastianLedererIcinga BastianLedererIcinga force-pushed the rule-filter-handling branch 8 times, most recently from d5fe2f5 to 3bf7acb Compare May 7, 2026 08:23
@nilmerg nilmerg self-requested a review May 11, 2026 13:36
Comment thread library/Notifications/Util/RuleSerializer.php Outdated
Copy link
Copy Markdown
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't look at the controller or suggestions yet.

Comment thread library/Notifications/Hook/V2/SourceHook.php Outdated
Comment thread library/Notifications/Hook/V2/SourceHook.php Outdated
Comment thread library/Notifications/Model/Source.php Outdated
Comment thread library/Notifications/Util/RuleSerializer.php Outdated
Comment thread library/Notifications/Util/RuleSerializer.php
Comment thread library/Notifications/Util/RuleSerializer.php Outdated
@BastianLedererIcinga BastianLedererIcinga force-pushed the rule-filter-handling branch 2 times, most recently from 015bfb8 to d8a98b8 Compare May 19, 2026 07:46
Comment thread library/Notifications/Common/SourceHookLocator.php Outdated
Comment thread application/controllers/EventRuleController.php Outdated
Comment thread application/forms/SourceForm.php Outdated
Comment thread library/Notifications/Hook/V2/SourceHook.php Outdated
Comment thread library/Notifications/Hook/V2/SourceHook.php Outdated
Comment thread library/Notifications/Hook/V2/SourceHook.php Outdated
Comment thread library/Notifications/Util/RuleSerializer.php Outdated
Comment thread library/Notifications/Web/Control/SearchEditor/RuleFilterSuggestions.php Outdated
Comment thread library/Notifications/Util/RuleSerializer.php Outdated
Comment thread application/controllers/EventRuleController.php Outdated
Comment thread application/controllers/EventRuleController.php Outdated
Comment thread application/controllers/EventRuleController.php Outdated
Comment thread library/Notifications/Util/RuleSerializer.php Outdated
Comment thread library/Notifications/Util/RuleSerializer.php Outdated
Copy link
Copy Markdown
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a unit test for the source hook locator and the rule serializer.

Comment thread application/controllers/EventRuleController.php Outdated
Comment thread application/controllers/EventRuleController.php Outdated
Comment thread test/php/library/Notifications/Common/SourceHookLocatorTest.php Outdated
nilmerg
nilmerg previously approved these changes May 21, 2026
Copy link
Copy Markdown
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for now. Let's see if we get Icinga/ipl-web#381 through before merging.

Comment thread library/Notifications/Util/RuleSerializer.php
Comment thread application/controllers/EventRuleController.php Outdated
Comment thread test/php/library/Notifications/Util/RuleSerializerTest.php Outdated
Allow serializing rules as JSON by providing a filter and the
JsonPaths for all used columns.
Introduce a new version of the `SourceHook` interface for V 1.0,
which contains all necessary funtions to ensure SearchEditors
for rules can be set up by notifications-web, and their filters
can be serialized in the way the daemon expects them.
`SourceHookLocator::forType()` is used to get the SourceHook
of the given type.
This allows `SourceForm` to accept text input for the source type.
Adjust the controller to rely on the new `V2\SourceHook` interface,
to set up the filter editor and serialize it's filter.
@nilmerg nilmerg merged commit 3345f7b into main May 22, 2026
13 checks passed
@nilmerg nilmerg deleted the rule-filter-handling branch May 22, 2026 14:15
nilmerg added a commit to Icinga/icingadb-web that referenced this pull request May 22, 2026
This PR adds a new `V2/Icinga2Source` hook implementation to integrate
with the `V2/SourceHook` interface from `icinga-notifications-web`.

### Supporting changes required by the hook:
- Introduce `QueryValuesProvider` — Decouples value suggestion logic
from `ObjectSuggestions` into a standalone Generator-based class. This
allows the hook's `getValueSuggestions()` to reuse the same suggestion
infrastructure without a `Suggestions` context.
- `QueryColumnsProvider`: Add `setShowRelationLabels()` setter — Exposes
the relation label display option so the hook can control it.
- `QueryColumnsProvider`: Make `collectFilterColumns()` protected — No
longer needs to be public; the override in `SearchControls` that called
it statically is removed as the optimization it provided is no longer
necessary.
- `ObjectSuggestions`: Remove label-to-path resolution for columns with
spaces to ensure consistency between the suggestions in the search bar
and suggestions provided by the `Icinga2Source`.

resolves: #1365
### Requires:
- Icinga/icinga-notifications-web#469
- #1375
- Icinga/ipl-orm#161
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla/signed CLA is signed by all contributors of a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle rule filter rendering and serialization on our own again

2 participants